HIVE-27006: Fix ParallelEdgeFixer by ngsg · Pull Request #4043 · apache/hive

ngsg · 2023-02-08T10:02:30Z

What changes were proposed in this pull request?

ParallelEdgeFixer refers to RowSchema when inverting columns and updates RuntimeValueInfo as well as SemiJoinBranchInfo.

Why are the changes needed?

Current ParallelEdgeFixer does not update RuntimeValueInfo while SemiJoinBranchInfo is updated.
Since TezCompiler refers to RuntimeValueInfo when adding SemiJoin edges into a Tez DAG, the inconsistency between RuntimeValueInfo and SemiJoinBranchInfo leads to the absence of SemiJoin edge in Tez runtime.

Another problem of ParallelEdgeFixer is incorrect result of colMappingInverseKeys().
In current implementation, colMappingInverseKeys() depends on Operator.getColumnExprMap(), but I found that this method sometimes returns an empty map although the Operator contains some columns. (Also the comment of this method says that it returns only key columns for RS and GBY operators.)
When this happens, ParallelEdgeFixer inserts a SEL operator without any column, and its child RS operator eventually fails due to Runtime error like below message.

Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: java.lang.RuntimeException: cannot find field _col0 from []
        at org.apache.hadoop.hive.ql.exec.ReduceSinkOperator.process(ReduceSinkOperator.java:384)
        at org.apache.hadoop.hive.ql.exec.Operator.forward(Operator.java:888)
        at org.apache.hadoop.hive.ql.exec.SelectOperator.process(SelectOperator.java:94)

Does this PR introduce any user-facing change?

No

How was this patch tested?

I tested the patch manually on cluster using the query described in JIRA and TPC-DS queries.

sonarqubecloud · 2023-02-17T03:17:27Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
No Duplication information

github-actions · 2023-04-19T00:20:42Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

sonarqubecloud · 2023-05-19T15:27:04Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
10 Code Smells

No Coverage information
No Duplication information

github-actions · 2023-07-24T00:21:52Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

kgyrtkirk · 2023-11-10T12:24:25Z

ql/src/test/queries/clientpositive/sharedwork_semi_2.q

+set hive.exec.max.dynamic.partitions.pernode=4000;
+set hive.exec.max.dynamic.partitions=10000;
+set hive.exec.parallel.thread.number=32;
+set hive.exec.parallel=false;


why do you need to set all these irrelevant options?

removed irrelevant configurations

kgyrtkirk · 2023-11-10T12:32:37Z

ql/src/test/results/clientpositive/perf/tpcds30tb/tez/query2.q.out

        Map 14 <- Map 6 (BROADCAST_EDGE), Union 12 (CONTAINS)
        Map 5 <- Map 6 (BROADCAST_EDGE), Union 2 (CONTAINS)
-        Map 6 <- Reducer 8 (BROADCAST_EDGE), Reducer 9 (BROADCAST_EDGE)
+        Map 6 <- Reducer 10 (BROADCAST_EDGE), Reducer 8 (BROADCAST_EDGE), Reducer 9 (BROADCAST_EDGE)


this seems odd to me: I wonder why the sudden need of Reducer 10 for Map 6 ?

there are no changes in Map 6

Map 6 refers only to RS_47 and RS_51 ; so its odd that it has 3 Reducers

Current ParallelEdgeFixer does not update RuntimeValueInformation(RVI) correctly. Because TezCompiler creates SemiJoin edges based on RVI, this issue leads to absence of some edges.

The edge between Map6 and Reducer10 is one of the disappeared edge. After SWO, Map6 has 2 incoming SemiJoin edges that come from the same reducer. So PEF inserts SEL-RS in order to prevent parallel edge, but it does not update RVI of the parent of the inserted SEL-RS. That's why previous plan does not contain an edge between Map6 and Reducer10.

I attached 3 operator graphs for the sake of your better understanding. All graphs are generated during TPCDS30TB-query2 test.
Before applying PEF:

After applying current PEF:

After applying modified PEF:

kgyrtkirk · 2023-11-10T12:38:01Z

ql/src/java/org/apache/hadoop/hive/ql/optimizer/SharedWorkOptimizer.java

+      boolean notTraverseable = !traverseableEdgeTypes.contains(opEdge.getEdgeType());
+      boolean notInvertible = (s instanceof ReduceSinkOperator) &&
+          !ParallelEdgeFixer.colMappingInverseKeys((ReduceSinkOperator) s).isPresent();

+      return notTraverseable || notInvertible;


I think its better to not change something which is not broken...

the previous version was eagerly avoiding to call PEF#colMIK in case the edge type was not matching - what if for some reason it starts throwing exceptions for irrelevant cases?

I reverted it.

ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java

deniskuzZ

LGTM +1, pendind tests
@ngsg, thank you for the contribution!

sonarqubecloud · 2023-11-22T13:40:48Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

No Coverage information
No Duplication information

The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

…an Haindrich, Denys Kuzmenko) Closes apache#4043

kgyrtkirk added the tests pending label Feb 8, 2023

ngsg changed the title ~~Fix ParallelEdgeFixer~~ HIVE-27006: Fix ParallelEdgeFixer Feb 8, 2023

kgyrtkirk added tests unstable and removed tests pending labels Feb 8, 2023

ngsg force-pushed the HIVE-27006-Fix-ParallelEdgeFixer branch from 5d4ff36 to 58f970d Compare February 16, 2023 10:03

kgyrtkirk added tests pending tests failed and removed tests unstable tests pending tests failed labels Feb 16, 2023

kgyrtkirk added tests passed and removed tests pending labels Feb 17, 2023

github-actions bot added the stale label Apr 19, 2023

github-actions bot closed this Apr 27, 2023

deniskuzZ reopened this May 19, 2023

deniskuzZ requested review from kasakrisz and zabetak May 19, 2023 10:52

kgyrtkirk added tests pending and removed tests passed labels May 19, 2023

kgyrtkirk added tests failed and removed tests pending labels May 19, 2023

deniskuzZ removed the stale label May 24, 2023

github-actions bot added the stale label Jul 24, 2023

github-actions bot closed this Aug 1, 2023

ayushtkn reopened this Sep 16, 2023

kgyrtkirk reviewed Nov 10, 2023

View reviewed changes

resolve requested changes

e095fd0

asf-ci-hive added tests pending tests unstable and removed tests passed tests pending labels Nov 13, 2023

restore intermediate RS creation in DPPOptimization; typo

d3cbcf0

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending labels Nov 20, 2023

deniskuzZ reviewed Nov 21, 2023

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Nov 21, 2023

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java Outdated Show resolved Hide resolved

minor

f3bc161

asf-ci-hive added tests pending and removed tests passed labels Nov 22, 2023

deniskuzZ reviewed Nov 22, 2023

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java Outdated Show resolved Hide resolved

minor

f38186b

asf-ci-hive added tests unstable tests pending and removed tests pending tests unstable labels Nov 22, 2023

deniskuzZ approved these changes Nov 22, 2023

View reviewed changes

asf-ci-hive added tests passed and removed tests pending labels Nov 22, 2023

deniskuzZ merged commit 5861b16 into apache:master Nov 23, 2023

tarak271 pushed a commit to tarak271/hive-1 that referenced this pull request Dec 19, 2023

HIVE-27006: Fix ParallelEdgeFixer (Seonggon Namgung, reviewed by Zolt…

557908a

…an Haindrich, Denys Kuzmenko) Closes apache#4043

Conversation

ngsg commented Feb 8, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

sonarqubecloud bot commented Feb 17, 2023

Uh oh!

github-actions bot commented Apr 19, 2023

Uh oh!

sonarqubecloud bot commented May 19, 2023

Uh oh!

github-actions bot commented Jul 24, 2023

Uh oh!

kgyrtkirk Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

ngsg Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

kgyrtkirk Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

ngsg Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

kgyrtkirk Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

ngsg Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deniskuzZ left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants